Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TAS-2525] 💄 Purchase CTA button improvement #1948

Merged
merged 13 commits into from
Nov 15, 2024

Conversation

AuroraHuang22
Copy link
Contributor

@AuroraHuang22 AuroraHuang22 commented Nov 13, 2024

  • - edition button
  • - cta button
  • - column left UI
  • - gift dialog
  • - discount + stock label
截圖 2024-11-13 下午3 03 42

截圖 2024-11-13 下午3 48 09

Copy link

AuroraHuang22 added a commit that referenced this pull request Nov 13, 2024
Comment on lines 31 to 40
<div class="text-like-green text-[16px] font-500">{{ name }}</div>
</div>
</div>

<div class="flex flex-co whitespace-nowrap">
<div v-if="discountInfo" class="text-like-green text-[12px]">
{{ discountInfo.originalPriceLabel }}
</div>
<div class="text-like-green text-[16px] font-500">
{{ priceLabel }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use v-text for content

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 28 to 30
<div>
<NFTStockLabel :stock="stock" />
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the extra div for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

</div>
</div>

<div class="flex flex-co whitespace-nowrap">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

Suggested change
<div class="flex flex-co whitespace-nowrap">
<div class="flex flex-col whitespace-nowrap">

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 33 to 35
<p class="ml-[8px]">
{{ $t('nft_edition_select_confirm_button_text_add_to_cart') }}
</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use v-text for content

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 6 to 8
<div class="text-[18px] font-500 text-dark-gray w-full">
{{ $t('nft_edition_select_section_label') }}
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use v-text for content

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/>
</template>

class="hidden laptop:block border-b-[1px] border-[#ebebebeb] w-full my-[16px]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is border with transparency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems not?
截圖 2024-11-13 下午2 19 19

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the hex value #ebebebeb?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AuroraHuang22 added a commit that referenced this pull request Nov 13, 2024
@AuroraHuang22 AuroraHuang22 marked this pull request as ready for review November 13, 2024 07:09
AuroraHuang22 added a commit that referenced this pull request Nov 13, 2024
Comment on lines 392 to 396
isAllSoldOut() {
return [this.formattedCollection].every(
item => item.stock === 0 || item.priceLabel === undefined
);
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why make an single item array? Just check this.formattedCollection without using .every() would do

},
data() {
// NOTE: If the selected item is out of stock, select another item.
const items = [...this.items];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why clone items?

AuroraHuang22 added a commit that referenced this pull request Nov 15, 2024
@williamchong williamchong merged commit 01bd5af into likecoin:develop Nov 15, 2024
1 check passed
Copy link

sentry-io bot commented Nov 26, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ NavigationDuplicated: Avoided redundant navigation to current location: "/zh-Hant/nft/class/likenft1mgehyarx3lft82g9zwv... new Promise(<anonymous>) View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants